🐛(backend) Fix unreachable exception handler for URLValidator#2172
Conversation
WalkthroughThe change modifies error handling in the CORS proxy URL validation logic by switching from catching Django REST Framework's Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
You are right, the exception is managed by the exception handler but not caught here. |
maboukerfa
force-pushed
the
fix/urlvalidator-exception-handling
branch
from
e1e9918 to
e6c0f30
Compare
Thank you! |
|
Thank you for your contribution. |
The exception block was never being executed because URLValidator raises django.core.exceptions.ValidationError, not drf.exceptions.ValidationError. Signed-off-by: Mohamed El Amine BOUKERFA <boukerfa.ma@gmail.com>
lunika
force-pushed
the
fix/urlvalidator-exception-handling
branch
from
e6c0f30 to
45abefb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/backend/core/api/viewsets.py`:
- Around line 2138-2141: The exception handler currently returns a stringified
ValidationError via {"detail": str(e)}; change it to return structured
validation data instead: extract the validation payload from the exception
(prefer e.message_dict if present, then e.messages, then e.detail) and use that
value as the "detail" in the drf.response.Response so clients get
machine-readable error structures; update the except block that catches
ValidationError in viewsets.py (the variable e and the drf.response.Response
call) to build and return this structured payload instead of str(e).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 428d53ed-5c76-474b-adee-00820d53188c
📒 Files selected for processing (2)
src/backend/core/api/viewsets.pysrc/backend/core/tests/documents/test_api_documents_cors_proxy.py
| except ValidationError as e: | ||
| return drf.response.Response( | ||
| {"detail": str(e)}, | ||
| status=drf.status.HTTP_400_BAD_REQUEST, |
There was a problem hiding this comment.
Return structured validation details instead of a stringified list
On Line 2140, {"detail": str(e)} produces a list-as-string payload ("['Enter a valid URL.']"). Prefer a structured detail to keep the API stable and machine-friendly.
💡 Proposed change
- except ValidationError as e:
+ except ValidationError as e:
return drf.response.Response(
- {"detail": str(e)},
+ {"detail": e.messages},
status=drf.status.HTTP_400_BAD_REQUEST,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except ValidationError as e: | |
| return drf.response.Response( | |
| {"detail": str(e)}, | |
| status=drf.status.HTTP_400_BAD_REQUEST, | |
| except ValidationError as e: | |
| return drf.response.Response( | |
| {"detail": e.messages}, | |
| status=drf.status.HTTP_400_BAD_REQUEST, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/backend/core/api/viewsets.py` around lines 2138 - 2141, The exception
handler currently returns a stringified ValidationError via {"detail": str(e)};
change it to return structured validation data instead: extract the validation
payload from the exception (prefer e.message_dict if present, then e.messages,
then e.detail) and use that value as the "detail" in the drf.response.Response
so clients get machine-readable error structures; update the except block that
catches ValidationError in viewsets.py (the variable e and the
drf.response.Response call) to build and return this structured payload instead
of str(e).
2 participants
Purpose
The exception block was never being executed because URLValidator raises django.core.exceptions.ValidationError, not drf.exceptions.ValidationError, so the except block was dead code.
Proposal
Replace the
drf.exceptions.ValidationErrorwith Django 'ValidationError'